Update handling of CSS border parts like border-color and add tests for that. (mathjax/MathJax#3490)#1408
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1408 +/- ##
==========================================
Coverage 86.50% 86.51%
==========================================
Files 340 340
Lines 85952 85987 +35
Branches 4816 3184 -1632
==========================================
+ Hits 74357 74392 +35
- Misses 11572 11595 +23
+ Partials 23 0 -23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
border parts like border-color and add tests for that. (mathjax/MathJax#3490)
| } | ||
| } | ||
| if (parts.length) { | ||
| if (parts.length > 1) { |
There was a problem hiding this comment.
Why is the style deleted if it has only one component?
There was a problem hiding this comment.
When there is only one part, it will be handled by a subpart. That is, if border soul be set to red, it is better handled as border-color: red. That was the the basis of the original issue, as border: red also resets the style and width, while border-color: red does not.
I'm not sure whether this should really be parts.length === 3 instead.
|
I've made the changes; can you re-review? |
This PR fixes the handling of some border CSS properties like
border-colorthat incorrectly ended up being translated toborderspecifications when they shouldn't. Its also updates the tests to check this.The test file adds a test for setting
border-color, which was the issue reported. It also moves the background check out of the border section and into a new border test section.The
ts/util/Styles.tsfile is modified as follows:The connection data adds a new
partsarray to list the sub-properties likeborder-colorthat modify other styles likeborder-top-colorbut that don't combine directly to theborderproperty. It also adds asubPartboolean to indicate a property likeborder-colorthat acts in that way. Previously, we usedcombine: nullto indicate this, but that was not a very clear indication.A new
combinePart()function is used forborder-colorand the other ones like it to handle the adjustments to the styles for those. This usescombineTRBL()to first change the properties likeborder-top-color, then callscombineChildren()to see if a combinedborderproperty can now be used, then callscombineSame()to see ifborder-colorcan be used in place ofborder-top-color, etc., and finally calls the newcombineParent()function described below.The
combineWSC()function is modified to only combine only if there is more than one of the three properties that is specified, soborder-color: redalone won't move toborder: red.The connection definitions are modified to mark
borderas having width, style, and color sub-parts, and the definitions for those now usecombine: combinePartandsubPart: true.The
cssTextoutput now checks whether a combined part property is available before including the more specific ones, so thatborder-top-colorwill not be included ifborder-coloris available.The
set()function is refactored to move the code that handles combining changes into parent properties into a separatecombineParent()function (since it is now needed in several places). It also now uses thesubPartconnection property rather thancombine === nullto test forborder-colorand similar properties, and in then calls the connection'scombine()function, which now points tocombinePart()for these border properties. Finally, after combining parents, if the property is a specific sub-part (likeborder-top-color), now the it is changed, we check to see if the other TRBL parts can be combined (e.g., asborder-color).The code for
combineParent()is taken from the oldset()definition, and simplified a bit (caching theStyles.connect[name]locally asconnect), and then adds code to remove sub-parts when the child properties have been combined (so ifborderis able to be used, theborder-colorand other sub-parts will not be needed).The rest of the changes are simplifications or comment clean-ups.
Resolves issue mathjax/MathJax#3490.